Repository navigation
Conversation
…of mutating the shared one
…as the Docker context
…to feat/push-validate-actor-json-paths
…to feat/push-validate-actor-json-paths
…to feat/push-validate-actor-json-paths
push reads through a symlink and uploads the content at the link's own path, so a link to a file outside the upload root builds on the platform. The realpath escape rule refused it; the uploaded files rule already rejects anything push does not send.
The worker copies inputSchema over input and outputSchema over output before resolving, so a schema path under the alias was never checked.
The worker treats an empty string as an unset field, so an empty value was wrongly reported as the .actor folder being a directory. A backslash passes path.resolve on Windows but is a filename character on the Linux worker, so it is refused before any resolution.
The lib test now builds its fixture through useTempPath, as the other lib tests do, adds a changelog file so the case-insensitive match is checked for all three listed fields, and covers the reason given when the uploaded file differs only in case. The command test restores the real process.exitCode next to the mocked copy.
checkActorPaths resolves the roots and builds the posix set of uploaded files once and hands them to every field, instead of each field recomputing them.
apify-factory
left a comment
apify-factory
left a comment
There was a problem hiding this comment.
Looks good, nothing blocking.
🏭 Factory review — ℹ️ not auto-approved
This change was reviewed but cannot be auto-approved: a gate failed, so the review cannot end in an approval.
| check | result |
|---|---|
| Static gates | ❌ 14/16 — base-branch, max-lines |
| Review findings | ✅ none |
| Reviewer escalation | ❌ The new check makes apify push fail on paths that used to push fine, based on platform worker rules this repo can't confirm (for example, a missing readme or dockerfile used to fall back to the default). |
Juror 1 — claude-sonnet-5-5, adversarial |
⏭️ skipped — a gate failed |
Why it was not auto-approved
Failed gates:
base-branch: base is feat/push-monorepo-preflight, required mastermax-lines: 646 changed lines (+644/-2), limit 200
The comments above are the review itself; this section only explains the approval decision. Push a change and request @apify-factory again to re-run it, or ask a human to review.
Reviewed e3edc7960 · workflow run
apify-factory
left a comment
apify-factory
left a comment
There was a problem hiding this comment.
Looks good, nothing blocking.
…to feat/push-validate-actor-json-paths
…to feat/push-validate-actor-json-paths
…actor-json-paths # Conflicts: # src/commands/actors/push.ts # src/lib/docker-context.ts
apify-service-account
added
the
tested
| if (typeof value === 'string' && value !== '') fields.push({ field, value, caseInsensitive }); | ||
| }; | ||
|
|
||
| for (const field of LISTED_FILE_FIELDS) add(field, config[field], true); |
There was a problem hiding this comment.
apify push checks dockerfile only when the key is present. as @apify/json_schemas@1.0.1 sets default to dockerfile
"dockerfile": {
"type": "string",
"default": "../Dockerfile"
},apify push succeed and then build uses the platform’s default image or fail (depending if ../Dockerfile exists or not)
Steps to reproduce
FAILED — open ../Dockerfile: no such file or directory
gh pr checkout 1503
pnpm install --frozen-lockfile
pnpm build
mkdir -p tmp/test1/.actor tmp/test1/docker && echo 'FROM node:22' > tmp/test1/docker/Dockerfile
echo 'FROM node:22' > tmp/test1/Dockerfile
cat > tmp/test1/.actor/actor.json <<'JSON'
{ "actorSpecification": 1, "name": "test1", "version": "0.0", "dockerContextDir": "../docker" }
JSON
cd tmp/test1 && ../../dist/apify.js pushSteps to reproduce
Dockerfile not found, using the default one.
gh pr checkout 1503
pnpm install --frozen-lockfile
pnpm build
mkdir -p tmp/test1/.actor tmp/test1/docker && echo 'FROM node:22' > tmp/test1/docker/Dockerfile
echo 'FROM node:22' > tmp/test1/Dockerfile
cat > tmp/test1/.actor/actor.json <<'JSON'
{ "actorSpecification": 1, "name": "test1", "version": "0.0", "dockerContextDir": "../docker" }
JSON
cd tmp/test1 && ../../dist/apify.js pushIn both scenarios push should fail locally
| dockerContextRoot: dockerContext.kind === 'within' ? dockerContext.contextRoot : undefined, | ||
| }); | ||
| if (pathProblems.length > 0) { | ||
| error({ message: formatActorPathProblems(pathProblems) }); |
There was a problem hiding this comment.
Question:
Now every path problem is treated as fatal, but it feels to much for readme and changlog, as now on platform they are (at least readme) resolved from default (from root) if not exists and build succeed.
|
one preexisting problem I found -> with inputSchema: "./custom_input.json":
we should resolve also this in followup issue. |
Summary
apify pushchecks none of the file-path fields in.actor/actor.json. The platform resolves them after the upload: a missing schema fails the build, and a missingdockerfile,readme, orchangelogonly logs a warning and falls back to the default, so the build goes green with the wrong image or README. This PR applies the worker's rules in the CLI before the Actor is looked up or written.Closes #1500. Part of apify/apify-core#28685. Stacked on #1502: the check runs after the
dockerContextDirresolver.Changes
src/lib/actor-paths.ts:checkActorPathsreturns one problem (field, value, resolved path, reason) per failing field;formatActorPathProblemsbuilds the message.push.tsprints it and exits with code 5.dockerfile,readme,changelog,input,output,webServerSchema, thestoragesschemas, plusinputSchemaandoutputSchema. The worker copies a set alias overinputoroutput, so the alias wins and the base key is skipped. Inline objects and empty strings (unset on the worker) are skipped..actor/, inside the upload root, an existing file, and among the files push uploads.dockerfile,readme, andchangelogmatch case-insensitively, as the worker finds them by a directory listing; schemas match exactly.dockerContextDirpoints inside the Actor folder,dockerfilemust lie inside that context: the worker runs BuildKit with--local dockerfile=<context dir>andfilename=<Dockerfile relative to the context>, so a Dockerfile outside it cannot be read by the build. This goes beyond the issue's checklist, because the same constraint applies to today's push.Tests
test/local/lib/actor-paths.test.ts: every rule, the aliases, empty strings, backslashes, symlinks (a dangling one and a symlinked Actor folder included), and the case mismatch reason.test/local/commands/push-actor-paths.test.ts: wrong paths stop push with exit code 5 before any Actor read or write,--jsonkeeps stdout clean, and valid paths push as usual.update-docs(no change), andtest:localpass, except the two[python]fixture tests that need a newer Python.Notes for reviewers
uploadRoot.--diris not mistaken for an escape.🤖 Generated with Claude Code